Add the base Python environments "watchers".#13735
Add the base Python environments "watchers".#13735ericsnowcurrently merged 8 commits intomicrosoft:masterfrom
Conversation
Codecov Report
@@ Coverage Diff @@
## master #13735 +/- ##
==========================================
+ Coverage 59.82% 59.87% +0.05%
==========================================
Files 675 678 +3
Lines 37799 37872 +73
Branches 5446 5455 +9
==========================================
+ Hits 22613 22676 +63
+ Misses 14027 14019 -8
- Partials 1159 1177 +18
Continue to review full report at Codecov.
|
ericsnowcurrently
force-pushed
the
pyenvs-component-base-watchers
branch
from
b9baba8 to
b5d60b7
Compare
kimadeline
left a comment
kimadeline
left a comment
There was a problem hiding this comment.
Looks good to me, but there are points I don't understand 😅
| * | ||
| * The same rule-of-thumb for subclassing `WatcherBase` applies here. | ||
| */ | ||
| export class BasicPythonEnvsWatcher extends WatcherBase<BasicPythonEnvsChangedEvent> { |
There was a problem hiding this comment.
In which use cases would we use a BasicPythonEnvsWatcher instead of a PythonEnvsWatcher? i.e why are we exporting it, is it just for tests?
There was a problem hiding this comment.
It's for the case that a watcher does not possibly provide non-basic info (currently only searchLocation). That said, it is somewhat of an artifact of an earlier design I was exploring, where the actual locators (not wrappers) were all basic and wrappers provided the non-basic info.
There isn't much cost to leaving BasicPythonEnvsWatcher for now, adding it later would be more painful than removing it later, and my gut is telling me that it will be useful later, so I'd like to leave it. We can circle back on removing it after we have refactored our existing locators. Is there any additional clarification I could put in a comment that would help avoid any confusion about why BasicPythonEnvsWatcher exists?
…f code navigation).
|
Kudos, SonarCloud Quality Gate passed!
|
4 participants
The watchers are the event-based part of locators.
(Note that 2/3 of this PR is new tests.)
[ ] Has a news entry file (remember to thank yourself!).[ ] Has sufficient logging.[ ] Has telemetry for enhancements.[ ] Test plan is updated as appropriate.[ ]package-lock.jsonhas been regenerated by runningnpm install(if dependencies have changed).[ ] The wiki is updated with any design decisions/details.